[OpenVINO] fix: Respect trust_remote_code in export and improve safety for multimodal configs FIXES #1668#1673
Conversation
MissLostCodes
changed the title
trust_remote_code in OpenVINO export and improve safety for multimodal configstrust_remote_code in OpenVINO export and improve safety for multimodal configs
MissLostCodes
changed the title
trust_remote_code in OpenVINO export and improve safety for multimodal configstrust_remote_code in export and improve safety for multimodal configs
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
@echarlaix @popovaan @rkazants From the failing CI logs, it appears that This fix introduces:
This fix ensures:
After these changes, the tests that were previously failing due to incorrect propagation of |
MissLostCodes
changed the title
trust_remote_code in export and improve safety for multimodal configstrust_remote_code in export and improve safety for multimodal configs FIXES ISSUE #1668
|
@echarlaix @rkazants @popovaan |
MissLostCodes
changed the title
trust_remote_code in export and improve safety for multimodal configs FIXES ISSUE #1668trust_remote_code in export and improve safety for multimodal configs FIXES #1668
|
@echarlaix @popovaan @rkazants
|
|
@rkazants @popovaan @echarlaix
Here's the detailed test result notebook |
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenVINO export flow to consistently honor trust_remote_code (to prevent unintended remote code execution) and adds defensive handling for certain multimodal configs during export.
Changes:
- Propagate
trust_remote_codethrough key OpenVINO export entry points and multimodal export config construction. - Make multimodal config handling more robust (e.g., guard access to
mm_vision_tower). - Add an early warning in task inference when
trust_remote_code=Falsefor Transformers models.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
optimum/exporters/openvino/__main__.py |
Adds a warning when remote code trust is disabled and ensures trust_remote_code is passed to AutoConfig.from_pretrained. |
optimum/exporters/openvino/convert.py |
Threads trust_remote_code into submodel/export-config selection, and conditionally passes it into relevant multimodal config constructors. |
optimum/exporters/openvino/model_configs.py |
Stops hardcoding trust_remote_code=True for multimodal vision-tower config loads; adds defensive hasattr checks. |
tests/openvino/test_genai.py |
Updates the set of architectures treated as requiring remote code in GenAI tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if export_model_type in {"llava-qwen2", "phi3_v"}: | ||
| config_kwargs["trust_remote_code"] = trust_remote_code | ||
|
|
There was a problem hiding this comment.
Why is trust_remote_code flag propagated here only for "llava-qwen2" and "phi3_v"? Maybe we need to do it unconditionally?
There was a problem hiding this comment.
other models do require trust_remote_code at the Transformers level but their corresponding OpenVINO config classes do not accept this parameter yet . So the condition is required
propagating trust_remote_code unconditionally, leads to :
TypeError: Qwen2VLOpenVINOConfig.__init__() got an unexpected keyword argument 'trust_remote_code'
TypeError: InternVLChatOpenVINOConfig.__init__() got an unexpected keyword argument 'trust_remote_code'
TypeError: MiniCPMVOpenVINOConfig.__init__() got an unexpected keyword argument 'trust_remote_code'
| "xglm", | ||
| "granitemoe", | ||
| "granite", | ||
| "llama4", | ||
| "zamba2", |
There was a problem hiding this comment.
why these changes are here? Looks not correct merge conflicts resolution
Please add tests to show that without 'trust_rmote_code' export of models do not work for certain models.
This test should check that error is raised for such models exporting wihtout this option. Quite useful test
| if len(only_onnx) > 0: | ||
| logger.warning(f"The following architectures export {only_onnx} is supported by ONNX but not OpenVINO") | ||
|
|
||
| @parameterized.expand(OV_MULTIMODAL_REMOTE_CODE_MODELS) |
There was a problem hiding this comment.
we have a list of all trust remote code models. May be we can parameterize this test using this list.
There was a problem hiding this comment.
But the models in that list donot support trust remote code propogation at openvino level , they handle it at hugging face level so running this test over all those models will throw TypeError for unexpected argument and fail these tests .
|
@popovaan, please take a look at this PR |
Summary
This PR ensures that the trust_remote_code flag is consistently respected across the OpenVINO export pipeline and improves robustness when handling multimodal model configurations.
Motivation
Some multimodal models (e.g. LLaVA, Phi-3 Vision, Qwen-VL) rely on custom code from their Hugging Face repositories.
Previously:
trust_remote_codewas not consistently propagatedA warning is added in
infer_task()whentrust_remote_code=False: This informs users that the model may require remote code appears before Transformers raises an exceptionDefensive handling of multimodal configs
Added check:
hasattr(config, "mm_vision_tower")Reason :
Behavior
Without --trust-remote-code
Output :
This model may require executing custom code from its repository. For security reasons, this is disabled by default. Please review the source and rerun with
--trust-remote-codeif needed.With --trust-remote-code
Output:
testing_colab_notebook
All tests relevant tests were passed
Docs already include related info for flag... no need to change
Fixes #1668